[pull] main from react:main - #640
Merged
Merged
Conversation
…er (#37315) Every server entry point that accepts a `signal` attached an abort listener to it and only ever removed that listener from inside the listener itself. On the success path the signal never aborts, so the listener stayed attached and its closure kept the whole `Request`, and therefore the entire rendered output, reachable for as long as the caller's signal lived. This matters most for composite signals from `AbortSignal.any()` and for timeout signals, because the runtime retains those for as long as they carry a non-weak abort listener, and releases them only when the last listener is removed or the signal aborts. A composite passed to `prerender()` therefore became a garbage collection root holding a finished render for the lifetime of the process. A plain `AbortController` signal is never retained that way, but it still keeps the render reachable for as long as the caller holds the controller. Each listener is now bound to a lifetime signal passed to `addEventListener`, so the runtime removes the listener as soon as that signal aborts and nothing has to track a teardown function. Flight reuses `request.cacheController`, which already aborts on a fatal error, at the completion of the flush loop (depends on #37342), and in `abort()`. Fizz has no equivalent, so it gains a `renderLifetimeController` that aborts at those same three points. `processReply` creates its controller only when a caller passes a signal, so a reply without one allocates nothing. Since `abort()` returns early once the request is past `OPEN`, removing the listener at those points cannot change observable behavior. The fifty-two copies of the listener block across the entry points collapse to a single `attachAbortSignal` call each. Binding the listener to the render also covers a cancelled stream, which calls `abort()` without the request ever reaching a terminal status, so a teardown driven by that status would have left the listener attached. Fizz ends the lifetime in `fatalError` rather than at the `CLOSING` to `CLOSED` transition, because a shell error rejects before the caller receives a stream. Nothing then consumes the request, it never closes, and a listener waiting for that transition would never come off. The two new controllers are aborted with an explicit reason. A call to `abort()` without one constructs an `AbortError` DOMException. Capturing the stack trace dominates that cost, and the cost grows with the depth of the stack, so every render and every reply would pay for an object that no code reads. `processReply` no longer returns its `abort` function, because that return value existed only so each `encodeReply` implementation could wire the signal up itself, and nothing uses it now that the wiring lives inside. A reply whose model settles synchronously gets no listener, since aborting it was already a no-op. The tests assert on the lifetime signal, because the runtime's removal does not go through `removeEventListener` and is therefore invisible to a patched signal. `ReactFlightDOMNode-test` asserts the removal itself with `getEventListeners` from `node:events`, which jsdom has no equivalent for. Two cases stay open. A request whose stream is neither consumed nor cancelled never ends, and a reply with a part that never settles never settles either, so both keep their listener.
A debug channel with a readable side lets the client fetch debug objects lazily. For example, React serializes each component's props into the debug model and defers the part of an object tree that exceeds the model's object limit. A deferred object stays retained, and its debug chunk stays pending, until the client asks for it or the channel closes. The render can therefore complete while such an object is still outstanding. When that happens, `flushCompletedChunks` closes the main stream, sets the request status to `CLOSED`, and returns before it reaches the block that releases the render's resources. Once the client closes the debug channel and the retained objects drop, a later flush does reach that block, but the cache controller is only aborted while the status is below `ABORTING`, and `CLOSED` is above it. The signal is therefore never aborted at all rather than merely released late, so anything that waits on `cacheSignal()` to clean up resources waits for the lifetime of the process. This change moves the cache controller abort above the debug stream bookkeeping. An empty pending chunk count already means the render is complete, and debug chunks carry development-only instrumentation rather than the render's output, so the cache signal can abort at that point no matter what the debug stream is still doing. The path that writes debug chunks on the main stream can now reach this code on several flushes, which is safe because aborting an aborted controller does nothing a second time. The taint queue cleanup stays where it is. A tainted typed array, `DataView` or blob is checked against the taint registry as its chunk is written, and such a write can happen long after the render completes, either because the client queried a deferred debug object or because a blob's stream resolved late. Moving it up would let those writes through unchecked, and it would fix nothing, because unlike the abort it never sat behind the status guard.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )